Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dilution factor fix #956

Merged
merged 3 commits into from
Aug 8, 2019
Merged

Conversation

marxwillia
Copy link
Contributor

No longer use the name 'w' because it is confused with atomic symbol for Tungsten.

'CSVY field descriptions exist without corresponding csv data'
if unsupported_columns != set():
logger.warning("The following columns are specified in the csvy"
"model file, but are IGNORED by TARDIS: %s"%(str(unsupported_columns)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"model file, but are IGNORED by TARDIS: %s"%(str(unsupported_columns)))
"model file, but are IGNORED by TARDIS: {0}".format(unsupported_columns)))

@wkerzendorf
Copy link
Member

and is this shifting the dilution factor into the code. it doesn't look like it.

@@ -562,8 +580,8 @@ def from_csvy(cls, config):

dilution_factor = None
if hasattr(csvy_model_data, 'columns'):
if 'w' in csvy_model_data.columns:
dilution_factor = csvy_model_data['w'].iloc[0:].to_numpy()
if 'dilution_factor' in csvy_model_data.columns:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wkerzendorf is this what you meant by "shifting the dilution factor into the code?" This is the change that loads the given dilution factor profile into the csvy model.

@wkerzendorf wkerzendorf merged commit e8c6963 into tardis-sn:master Aug 8, 2019
atharva-2001 pushed a commit to atharva-2001/tardis that referenced this pull request Oct 1, 2021
* Replaced 'w' with 'dilution_factor' in csvy model

* Added Warning for unsupported CSVY columns

* Changed supported column checks to use Sets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants